Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update setOrder plugins signature #57

Merged
merged 1 commit into from
Aug 7, 2024
Merged

Update setOrder plugins signature #57

merged 1 commit into from
Aug 7, 2024

Conversation

ryanisn
Copy link
Contributor

@ryanisn ryanisn commented Aug 7, 2024

Despite magento documented desc as string in phpdoc, it is not typed in param signature. In one of the elasticsuite plugin, it passes array to the setOrder
https://github.com/Smile-SA/elasticsuite/blob/affec096d16194addea273dfa35145a42da4213a/src/module-elasticsuite-virtual-category/Plugin/Widget/ProductsListPlugin.php#L152
as you can see the array is assembled here
https://github.com/Smile-SA/elasticsuite/blob/affec096d16194addea273dfa35145a42da4213a/src/module-elasticsuite-virtual-category/Model/Widget/SortOrder/SkuPosition/Builder.php#L63

So to maintain compatibility with elasticsuite for now, we have to remove the string type, other wise it breaks website if there is a product list widget created.

@what-the-diff What The Diff
Copy link

what-the-diff bot commented Aug 7, 2024

PR Summary

  • Refactored variable type declaration in CollectionPlugin.php
    The developers revised the type specification for the variable $dir, within the beforeSetOrder and aroundSetOrder methods in CollectionPlugin.php. Previously, this was exclusively stated to be a string; now, this declaration has been removed to allow for greater flexibility in what types the variable can accept. This could, in turn, lead to enhanced versatility and overall system optimization.

@tuyennn
Copy link
Owner

tuyennn commented Aug 7, 2024

Thx @ryanisn for your contribution

@tuyennn tuyennn merged commit abbb098 into tuyennn:master Aug 7, 2024
4 of 5 checks passed
@tuyennn tuyennn added the compatibility Compatibility with 3rd party label Aug 7, 2024
@ryanisn
Copy link
Contributor Author

ryanisn commented Aug 7, 2024

Awesome, thanks for the quite PR merge!
Can you tag it with a new version so I can install it?

@tuyennn
Copy link
Owner

tuyennn commented Aug 8, 2024

Here https://github.com/tuyennn/magento2-outofstockatlast/releases/tag/v1.1.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
compatibility Compatibility with 3rd party
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@ryanisn @tuyennn